mem: Allow overriding the default buffer pool.#8806
Conversation
The default buffer pool only has a only a few tiers, which hurts performance in some applications. In, it was decided that instead of adding more tiers to the default pool, we should allow changing the default pool.
|
I didn't know if you |
vanja-p
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8806 +/- ##
==========================================
- Coverage 83.30% 83.00% -0.31%
==========================================
Files 418 414 -4
Lines 33004 32779 -225
==========================================
- Hits 27493 27207 -286
- Misses 4102 4115 +13
- Partials 1409 1457 +48
🚀 New features to boost your workflow:
|
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
I can't figure out how to get the PR validation to pass. My initial PR title didn't start with "mem:", but now it does. |
eshitachandwani
added
the
Type: Performance
|
@vanja-p can you check if the test failing is related to the changes or not? |
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@eshitachandwani, looks like it's not related to my change. Rerunning them passed, and also, my 2 new lines don't have any coverage. |
I think it's better to move |
arjan-bal
added
the
Area: RPC Features
Done. |
arjan-bal
left a comment
arjan-bal
left a comment
There was a problem hiding this comment.
Just one minor comment; otherwise, LGTM. Adding a second reviewer.
| // can't be changed otherwise. The provided buffer pool must be non-nil. This | ||
| // must be called before creating any grpc clients or servers. The default value | ||
| // is mem.DefaultBufferPool. | ||
| func SetDefaultBufferPool(bufferPool mem.BufferPool) { |
There was a problem hiding this comment.
Instead of requiring users to set this before making RPCs, we recommend calling these functions within an init() function. This prevents data races that can occur when multiple packages invoke the same method simultaneously. For example, the balancer.Register function includes the following requirement:
NOTE: this function must only be called during initialization time (i.e., in an init() function) and is not thread-safe...
Can you please add a similar note here?
internal/leakcheck/leakcheck.go
Outdated
| defaultPool := mem.DefaultBufferPool() | ||
| globalPool.Store(&defaultPool) | ||
| (internal.SetDefaultBufferPoolForTesting.(func(mem.BufferPool)))(&globalPool) | ||
| (internal.SetDefaultBufferPool.(func(mem.BufferPool)))(&globalPool) |
There was a problem hiding this comment.
Nit: Can the extra parenthesis be avoided? Can this be
internal.SetDefaultBufferPool.(func(mem.BufferPool))(&globalPool) ?
4 participants
The default buffer pool only has a only a few tiers, which hurts performance in some applications. In #8770, it was decided that instead of adding more tiers to the default pool, we should allow changing the default pool.
RELEASE NOTES: